Skip to content

chore: update crypto public key#2042

Merged
MonaaEid merged 10 commits into
hiero-ledger:mainfrom
MonaaEid:chore/update-crypto-public-key
Apr 25, 2026
Merged

chore: update crypto public key#2042
MonaaEid merged 10 commits into
hiero-ledger:mainfrom
MonaaEid:chore/update-crypto-public-key

Conversation

@MonaaEid

Copy link
Copy Markdown
Contributor

Description:
This PR introduces DER encoding helpers and compressed DER export support for ECDSA secp256k1 public keys, along with updates to the key type alias and improvements to key utility handling.The most important changes are:

  • Added DER encoding helper methods (_encode_der_length, _encode_der_oid, _encode_der_sequence, _encode_der_bit_string) and a method to export ECDSA secp256k1 public keys in compressed DER SPKI format (to_bytes_der_ecdsa_compressed) to PublicKey, with corresponding string export (to_string_der_ecdsa_compressed). (src/hiero_sdk_python/crypto/public_key.py)

  • Changed the Key type alias in key_utils.py to refer to the SDK's base Key class

  • Added unit tests for DER encoding helpers.

Related issue(s):
Fixes #2041

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@codecov

codecov Bot commented Mar 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.11321% with 1 line in your changes missing coverage. Please review.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2042      +/-   ##
==========================================
+ Coverage   93.68%   93.70%   +0.01%     
==========================================
  Files         145      145              
  Lines        9426     9470      +44     
==========================================
+ Hits         8831     8874      +43     
- Misses        595      596       +1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown

Hi, this is WorkflowBot.
Your pull request cannot be merged as it is not passing all our workflow checks.
Please click on each check to review the logs and resolve issues so all checks pass.
To help you:

@MonaaEid
MonaaEid force-pushed the chore/update-crypto-public-key branch from fa52b43 to 9ac3442 Compare March 30, 2026 00:22
@MonaaEid
MonaaEid marked this pull request as ready for review March 30, 2026 00:59
@MonaaEid
MonaaEid requested review from a team as code owners March 30, 2026 00:59
@MonaaEid
MonaaEid requested review from Akshat8510, Copilot and exploreriii and removed request for Copilot March 30, 2026 00:59
@coderabbitai

coderabbitai Bot commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Added DER/X.690 encoding helpers and two PublicKey serializers to export compressed ECDSA (secp256k1) SubjectPublicKeyInfo (DER/hex); widened SDK Key alias and adjusted key_to_proto dispatch; added extensive unit tests for DER helpers and ECDSA export; plus numerous CI/workflow, issue-template, and docs updates/deletions.

Changes

Cohort / File(s) Summary
Crypto — DER encoding & exports
src/hiero_sdk_python/crypto/public_key.py
Added internal DER helpers (_encode_vlq, _encode_der_length, _encode_der_oid, _encode_der_sequence, _encode_der_bit_string) and two public methods: to_bytes_der_ecdsa_compressed() and to_string_der_ecdsa_compressed() that build SPKI DER from compressed SEC1 points; non-ECDSA keys raise ValueError.
Key utils
src/hiero_sdk_python/utils/key_utils.py
Broadened exported Key alias to include the SDK base Key type (SdkKey), changed if not key:if key is None:, and dispatches to_proto_key() for SDK Key instances; updated TypeError message.
Tests — DER & ECDSA
tests/unit/keys_public_test.py, tests/unit/test_transaction_response.py, tests/unit/transaction_response_test.py
Added extensive unit tests for VLQ/DER length/OID/SEQUENCE/BIT STRING encoders and ECDSA compressed DER/SPKI export (including round-trip and error cases); reorganized/added TransactionResponse tests and minor test refactors.
Changelog
CHANGELOG.md
Inserted unreleased entries documenting DER encoding helpers, compressed ECDSA DER export, and key alias update; added notes about test and docs/workflow changes.
New workflow & scripts
.github/workflows/release-pr-coderabbit-gate.yml, .github/scripts/release-pr-coderabbit-gate.js
Added a workflow and script to post a single CodeRabbit release-review prompt comment on release PRs with deduplication via a hidden marker.
Bot scripts & tests
.github/scripts/bot-inactivity-unassign.sh, .github/scripts/test-bot-inactivity-unassign.sh
Added label-check skip for discussion in inactivity-unassign script; added a large Bash test harness that mocks gh for multiple scenarios.
Issue templates
.github/ISSUE_TEMPLATE/*
Added several new issue-form templates (good-first, beginner, intermediate, advanced variants) and removed multiple older template files; some templates replaced/renamed.
Workflows — pins, renames & additions
.github/workflows/**/*.yml
Widespread workflow edits: bumped pinned action SHAs (step-security/harden-runner, actions/github-script, others), added new workflows (CodeQL, release PR gate), renamed/retitled many workflows, added concurrency controls, trigger/path changes, and several workflow logic simplifications (e.g., sync-issue-labels).
Sync labels & compute rewrite
.github/workflows/sync-issue-labels-compute.yml, .github/workflows/sync-issue-labels-add.yml
Rewrote label-compute logic to parse PR body for linked issues, fetch issue labels, and emit labels.json; replaced downstream add-job flow with a single GitHub Script step consuming artifact.
Docs removals & link redirects
docs/**, CONTRIBUTING.md, README.md
Removed many maintainer and workflow docs, moved/rewired developer guide links to external collaboration-hub GitHub URLs, updated several docs to new link targets and trimmed content (multiple deletions).
Team & metadata
docs/team.md, CHANGELOG.md
Team table pruned and reformatted; changelog unreleased entries added (see above).
Spam list & small files
.github/spam-list.txt, .github/coderabbit/release-pr-prompt.md
Added spam entry (ashrafzunaira18) and a new release-review prompt document for the coderabbit workflow.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 4

❌ Failed checks (3 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR fails to meet critical advanced-issue prerequisites. The issue #2041 requires ≥10 completed intermediate issues, ≥3-4 months active contribution history, and deep architectural expertise. These mandatory contributor qualifications are not evidenced in the PR context. Additionally, the timeline red-flag (completing advanced work in days vs. the stated ~1 month requirement) suggests insufficient vetting of prerequisites before implementation began.
Out of Scope Changes check ⚠️ Warning Extensive out-of-scope changes detected: 40+ GitHub Actions workflow files updated (harden-runner/github-script version bumps), 20+ documentation files deleted/refactored (guides, maintainer docs, contributor resources), GitHub issue templates reorganized (4 deleted, 4 new), changelog/team metadata changes. These are unrelated to the core objective of adding DER encoding and compressed SPKI export for ECDSA keys [#2041].
Docstring Coverage ⚠️ Warning Docstring coverage is 57.81% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'chore: update crypto public key' is vague and generic, using non-descriptive phrasing that doesn't convey the specific cryptographic improvements (DER encoding, compressed SPKI export). Consider a more specific title like 'feat: add compressed DER SPKI export for ECDSA secp256k1' or 'feat: implement DER encoding helpers and ECDSA compressed export' to better communicate the primary changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description is clearly related to the changeset, outlining the DER encoding helpers, compressed SPKI export methods, key type alias updates, and unit tests added in this PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📋 Issue Planner

Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).

View plan for ticket: #2041

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch chore/update-crypto-public-key

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3217c3bb-9b94-4e8a-929b-8cb77048a2b3

📥 Commits

Reviewing files that changed from the base of the PR and between 5b57ef3 and 9c2bf3c.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • src/hiero_sdk_python/crypto/public_key.py
  • src/hiero_sdk_python/utils/key_utils.py
  • tests/unit/keys_public_test.py

Comment thread src/hiero_sdk_python/crypto/public_key.py
Comment thread src/hiero_sdk_python/utils/key_utils.py Outdated
Comment thread tests/unit/keys_public_test.py Outdated
@MonaaEid MonaaEid added reviewer: maintainer PR needs a review from the maintainer team and removed status: in initial development labels Mar 30, 2026

@AntonioCeppellini AntonioCeppellini left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @MonaaEid :D, i think this should work fine, i just suggested a couple of cases in tests

Comment thread tests/unit/keys_public_test.py Outdated
Copilot AI review requested due to automatic review settings April 6, 2026 19:20
@codacy-production

codacy-production Bot commented Apr 6, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@MonaaEid
MonaaEid removed the request for review from Copilot April 6, 2026 19:22

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
tests/unit/keys_public_test.py (1)

288-313: 🛠️ Refactor suggestion | 🟠 Major

Missing test for to_string_der_ecdsa_compressed().

The byte exporter is well-tested, but the public hex wrapper to_string_der_ecdsa_compressed() is not exercised. A regression in casing, prefixing, or delegation would still pass this suite.

🧪 Suggested test
+def test_to_string_der_ecdsa_compressed_matches_bytes(ecdsa_keypair):
+    _, pub = ecdsa_keypair
+    public_key = PublicKey(pub)
+
+    value = public_key.to_string_der_ecdsa_compressed()
+
+    assert isinstance(value, str)
+    assert value == public_key.to_bytes_der_ecdsa_compressed().hex()

As per coding guidelines, "PRIORITY 1 - Protect Against Breaking Changes: Assert return types where relevant" and "PRIORITY 3 - Comprehensive Coverage: Unit tests should be extensive."


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 102a2c9d-26a5-4d3b-9540-aff67fb4ea42

📥 Commits

Reviewing files that changed from the base of the PR and between 9c2bf3c and 2bd46b8.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • tests/unit/keys_public_test.py

Comment thread tests/unit/keys_public_test.py Outdated
@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown

Hello, this is the OfficeHourBot.

This is a reminder that the Hiero Python SDK Office Hours are scheduled in approximately 4 hours (14:00 UTC).

This session provides an opportunity to ask questions regarding this Pull Request.

Details:

Disclaimer: This is an automated reminder. Please verify the schedule here for any changes.

From,
The Python SDK Team

Comment thread tests/unit/keys_public_test.py Outdated
@exploreriii exploreriii added status: ready for review step: 1st 1st stage of the review approval process and removed status: ready for review labels Apr 14, 2026
Copilot AI review requested due to automatic review settings April 16, 2026 16:53
@MonaaEid
MonaaEid requested a review from a team as a code owner April 16, 2026 16:53
@MonaaEid
MonaaEid requested a review from jeromy-cannon April 16, 2026 16:53
@exploreriii

Copy link
Copy Markdown
Contributor

Converting to draft while changes requested above

@exploreriii
exploreriii marked this pull request as draft April 21, 2026 16:11
@github-actions

Copy link
Copy Markdown

Hi, this is WorkflowBot.
Your pull request cannot be merged as it is not passing all our workflow checks.
Please click on each check to review the logs and resolve issues so all checks pass.
To help you:

@MonaaEid
MonaaEid force-pushed the chore/update-crypto-public-key branch from 1435de8 to 7452cfa Compare April 21, 2026 21:18
Signed-off-by: MonaaEid <monaa_eid@hotmail.com>
@MonaaEid
MonaaEid force-pushed the chore/update-crypto-public-key branch from 7452cfa to 01968e6 Compare April 22, 2026 22:48
@MonaaEid
MonaaEid marked this pull request as ready for review April 22, 2026 23:06
@MonaaEid
MonaaEid requested review from Dosik13 and manishdait April 22, 2026 23:26
Comment thread src/hiero_sdk_python/utils/key_utils.py Outdated
Signed-off-by: MonaaEid <monaa_eid@hotmail.com>
@MonaaEid
MonaaEid force-pushed the chore/update-crypto-public-key branch from 9453aeb to 06c217e Compare April 23, 2026 07:13
@MonaaEid
MonaaEid requested a review from Dosik13 April 23, 2026 07:15
@aceppaluni

Copy link
Copy Markdown
Contributor

@MonaaEid Can you resolve the open conversations to help speed up review time?

Thank you so much!

@exploreriii exploreriii added step: 1st 1st stage of the review approval process reviewer: maintainer PR needs a review from the maintainer team labels Apr 24, 2026
@exploreriii exploreriii added step: 2nd second stage of the review approval process and removed step: 1st 1st stage of the review approval process labels Apr 25, 2026
Comment thread src/hiero_sdk_python/utils/key_utils.py
@MonaaEid
MonaaEid requested a review from manishdait April 25, 2026 13:48
@MonaaEid
MonaaEid merged commit c5baa77 into hiero-ledger:main Apr 25, 2026
33 of 35 checks passed
@MonaaEid
MonaaEid deleted the chore/update-crypto-public-key branch April 25, 2026 14:13
@MonaaEid MonaaEid added this to the v0.2.6 milestone Apr 25, 2026
parvninama pushed a commit to parvninama/hiero-sdk-python that referenced this pull request Apr 25, 2026
Signed-off-by: MonaaEid <monaa_eid@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lang: python Uses Python programming language reviewer: maintainer PR needs a review from the maintainer team skill: advanced requires knowledge of multiple areas in the codebase without defined steps to implement or examples step: 2nd second stage of the review approval process

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Advanced]: Implemente compressed DER SPKI export methods for ECDSA secp256k1

7 participants